[MRG] Update empty-room matching to look for and prioritize same-session recordings with task "noise" (closes #1354) #1364
Conversation
|
Hello! 👋 Thanks for opening your first pull request here! |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
sappelhoff
left a comment
sappelhoff
left a comment
There was a problem hiding this comment.
Thanks! Could you please also follow the steps in the first-time-contributors list? https://github.com/mne-tools/mne-bids/blob/main/CONTRIBUTING.md#instructions-for-first-time-contributors
I think it would be a good idea to write a detailed changelog item for this feature, too!
|
@berkgercek please also note that I have merged |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1364 +/- ##
==========================================
+ Coverage 97.42% 97.43% +0.01%
==========================================
Files 40 40
Lines 8922 8966 +44
==========================================
+ Hits 8692 8736 +44
Misses 230 230 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Stefan Appelhoff <[email protected]>
|
Ok, I've updated the changelog and added myself as a contrib! The test coverage has decreased with this PR because I added functionality to find split-file ER recordings. I would like to cover that section of the code too but there's no easy way using Edit: I also am not entirely sure why the docs are failing to build... |
sappelhoff
left a comment
sappelhoff
left a comment
There was a problem hiding this comment.
Great work, thanks a lot @berkgercek
|
🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪 |
…ion recordings with task "noise" (closes mne-tools#1354) (mne-tools#1364) * Changed empty-room matching logic to prioritize task-noise files * Simple test for noise task * Tests for the split noise file case and fallback to sub-emptyroom * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix line length and clean up after test * Work around the weird multiple-match behavior of path.match() * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Better handling of "run" set in the base path for ER search * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Remove an extraneous print statement * Update mne_bids/tests/test_path.py Co-authored-by: Stefan Appelhoff <[email protected]> * Updated changelog, added self to author list per new contrib guide * Update CITATION.cff to remove name from original pub * Update doc/authors.rst to fix doc build Co-authored-by: Daniel McCloy <[email protected]> * Added test logic for new code * Line length * Cover an unrelated error to bring up % * correct author order --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Stefan Appelhoff <[email protected]> Co-authored-by: Daniel McCloy <[email protected]>
4 participants
PR Description
task-noisefiles in same session #1354To resolve the lack of "noise" task matching mentioned in #1354 I've implemented logic that prioritizes files found in the same session with
task-noiseoversub-emptyroomrecordings. See the proposal in the mentioned issue for the details of the logic.Merge checklist
Maintainer, please confirm the following before merging.
If applicable: